Skip to content

chore(frontend): shadcn v4 upgrade, Alby theme polish, hub logo#2155

Open
stackingsaunter wants to merge 15 commits intogetAlby:masterfrom
stackingsaunter:chore/shadcn-ui-upgrade
Open

chore(frontend): shadcn v4 upgrade, Alby theme polish, hub logo#2155
stackingsaunter wants to merge 15 commits intogetAlby:masterfrom
stackingsaunter:chore/shadcn-ui-upgrade

Conversation

@stackingsaunter
Copy link
Contributor

@stackingsaunter stackingsaunter commented Mar 20, 2026

Summary

shadcn / Tailwind v4

  • Refresh vendored shadcn/ui primitives (components.json, Tailwind v4–compatible setup).
  • Sync frontend dependencies (radix-ui, lucide-react, etc.).
  • Preserve Hub-specific patterns: split buttonVariants / badgeVariants, hybrid tooltip + TouchProvider, custom variants, sonner tied to document dark class.
  • Card keeps shadow-sm to match current master default.

Alby theme & branding

  • Primary button: lighter gradient, #ffdf6f ring, stacked focus-visible ring with theme --ring.
  • Neutrals: achromatic grays (replace taupe); dark primary button label matches light.
  • Figtree for Alby via @fontsource-variable/figtree; Tailwind font-sans follows per-theme --app-font-sans (default.css / index.css).

Hub wordmark (AlbyHubLogo)

  • New compact SVG; default + alby use design light/dark fills (#202020 / #ffc800 light; white / #ffe480 dark); other themes use monochrome currentColor.
  • invert forces dark-surface variant on default/alby (e.g. intro column).
  • Sidebar / fullscreen layouts keep height matched to the previous wide logo.

Link-as-button styling

  • ExternalLink / LinkButton / ExternalLinkButton forward data-slot / data-variant so themed primary button CSS applies.

Test plan

  • cd frontend && yarn lint
  • Smoke: dialogs, sheet, sidebar, forms, tables, toasts (shadcn)
  • Smoke: default + Alby light/dark; another theme + dark mode; intro two-column + sidebar logo

Summary by CodeRabbit

  • New Features

    • Added size options for Avatar and Switch; dialog and sheet can optionally show a close button.
    • New badge variant system and additional popover/tooltip header/title/description components.
  • Improvements

    • Updated fonts and theme palette (oklch) for richer typography and color; refined logo for monochrome and height-based sizing.
    • Enhanced toasts with custom icons and styling.
  • Bug Fixes

    • Deduplicated and simplified form field error messages.

- Refresh vendored UI primitives from shadcn registry (radix-ui, lucide, etc.)
- Fix components.json tailwind.config for Tailwind v4 CLI
- Preserve Hub splits: buttonVariants, badgeVariants, hybrid tooltip + TouchProvider
- Keep custom alert/badge variants; sonner uses document dark class
- Remove duplicate use-mobile.ts (use use-mobile.tsx only)
- Card keeps shadow-sm to match pre-change product default

Made-with: Cursor
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026

📝 Walkthrough

Walkthrough

Consolidates Radix imports to the unified radix-ui package, adds/new defaults for fonts and theme tokens (OKLCH palette), refactors many UI components' props and className compositions, introduces new UI subcomponents, updates package dependencies, and removes the useIsMobile hook.

Changes

Cohort / File(s) Summary
Package & Repo Config
frontend/package.json, frontend/components.json
Added radix-ui, font packages, date-fns; bumped lucide-react/react-day-picker; pinned Yarn via packageManager; cleared tailwind.config in components.json.
Radix Import Migration (multiple UI files)
frontend/src/components/ui/... (accordion, alert, avatar, badge, breadcrumb, button, checkbox, dialog, dropdown-menu, label, navigation-menu, popover, progress, radio-group, select, separator, sheet, sidebar, switch, tabs, tooltip, etc.)
Replaced many @radix-ui/react-* imports with radix-ui named exports; adjusted Slot usage to Slot.Root where applicable; reordered/normalized Tailwind class strings across components.
Component API & Behavior Changes
alert-dialog.tsx, avatar.tsx, badge.tsx, button.tsx, select.tsx, sheet.tsx, dialog.tsx, switch.tsx, tabs.tsx, popover.tsx, tooltip.tsx
Added/changed props and exports: AlertDialogContent size, AlertDialogMedia, Avatar size and AvatarBadge/Group/Count, Badge defaults and externalized badgeVariants, Button defaults/data-attributes, SelectContent position/align defaults, SheetContent/DialogFooter showCloseButton, Switch size, Tabs rewritten components/variants, new Popover/Tooltip subcomponents and provider behavior.
Styling & Variant Utilities
frontend/src/components/ui/buttonVariants.tsx, badgeVariants.tsx, button.tsx, calendar.tsx
Expanded button size variants, extracted badgeVariants via cva, switched calendar day button to use app Button component, adjusted numerous class-variance strings and ordering.
Theming & Fonts
frontend/src/themes/alby.css, frontend/src/themes/default.css, frontend/src/themes/index.css, frontend/src/fonts.css, frontend/src/main.tsx
Switched theme tokens to OKLCH, added many design tokens (sidebar, chart, logo), changed radius and button layer styles, added fontsource imports, removed local @font-face rules and removed fonts.css import from main.
Minor UI Adjustments & Export Ordering
card.tsx, carousel.tsx, checkbox.tsx, command.tsx, field.tsx, input.tsx, pagination.tsx, popover.tsx, radio-group.tsx, skeleton.tsx, table.tsx, textarea.tsx, stepper.tsx, circle-progress.tsx
Reordered Tailwind utility tokens, deduped FieldError messages and changed guard logic, switched some use client directives, changed export ordering in multiple modules; minor refactors (e.g., stable sidebar skeleton width).
Icon & Layout Usage
frontend/src/components/icons/AlbyHubLogo.tsx, AppSidebar.tsx, layouts/TwoColumnFullScreenLayout.tsx
AlbyHubLogo: invertmonochrome prop, SVG restructured to use CSS variables; updated usages to height-based sizing.
Hook Removal
frontend/src/hooks/use-mobile.ts
Removed useIsMobile hook implementation.
Custom Link/Button adjustments
frontend/src/components/ui/custom/external-link-button.tsx, link-button.tsx
Added data-slot="button" and data-variant attributes for consistent styling hooks.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • rolznz

Poem

🐰 Hopped through imports, tidy and spry,

Radix gathered under one sky,
Colors shifted to OKLCH delight,
Fonts and tokens dressed so bright—
A little rabbit danced in the code tonight.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.80% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: shadcn v4 upgrade, Alby theme polish, and hub logo updates.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/package.json (1)

25-41: ⚠️ Potential issue | 🟠 Major

Complete the Radix UI migration or remove the consolidated package.

The PR adds the consolidated radix-ui package (line 54) while individual @radix-ui/* packages (lines 25-41) remain. However, the migration is incomplete—two files still import from individual packages:

  • frontend/src/components/stepper.tsx
  • frontend/src/components/ui/custom/circle-progress.tsx

Either migrate these imports to the consolidated package and remove the individual dependencies, or revert the radix-ui addition to maintain consistency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/package.json` around lines 25 - 41, The package.json shows both the
consolidated "radix-ui" package and many individual "@radix-ui/*" packages;
resolve this by either (A) migrating the two remaining imports in
frontend/src/components/stepper.tsx and
frontend/src/components/ui/custom/circle-progress.tsx to import from "radix-ui"
and then remove the now-unneeded individual "@radix-ui/*" entries from
package.json, or (B) revert/remove the consolidated "radix-ui" dependency and
keep the existing individual "@radix-ui/*" entries, ensuring import paths in
stepper.tsx and circle-progress.tsx match the chosen approach; update
package.json and run install to verify no unresolved imports.
🧹 Nitpick comments (2)
frontend/src/components/ui/command.tsx (1)

53-53: Optional: extract the long Command class string for maintainability.

The inline class list is quite dense; moving it to a local const (or cva) would make future edits less error-prone.

Refactor sketch
+const commandDialogContentClass =
+  "**:data-[slot=command-input-wrapper]:h-12 [&_[cmdk-group-heading]]:px-2 [&_[cmdk-group-heading]]:font-medium [&_[cmdk-group-heading]]:text-muted-foreground [&_[cmdk-group]]:px-2 [&_[cmdk-group]:not([hidden])_~[cmdk-group]]:pt-0 [&_[cmdk-input-wrapper]_svg]:h-5 [&_[cmdk-input-wrapper]_svg]:w-5 [&_[cmdk-input]]:h-12 [&_[cmdk-item]]:px-2 [&_[cmdk-item]]:py-3 [&_[cmdk-item]_svg]:h-5 [&_[cmdk-item]_svg]:w-5";
...
-        <Command className="**:data-[slot=command-input-wrapper]:h-12 [&_[cmdk-group-heading]]:px-2 [&_[cmdk-group-heading]]:font-medium [&_[cmdk-group-heading]]:text-muted-foreground [&_[cmdk-group]]:px-2 [&_[cmdk-group]:not([hidden])_~[cmdk-group]]:pt-0 [&_[cmdk-input-wrapper]_svg]:h-5 [&_[cmdk-input-wrapper]_svg]:w-5 [&_[cmdk-input]]:h-12 [&_[cmdk-item]]:px-2 [&_[cmdk-item]]:py-3 [&_[cmdk-item]_svg]:h-5 [&_[cmdk-item]_svg]:w-5">
+        <Command className={commandDialogContentClass}>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/components/ui/command.tsx` at line 53, The Command component's
long inline className string should be extracted to a named constant (e.g.,
COMMAND_BASE_CLASSES or commandClasses) or a cva utility to improve
maintainability; locate the <Command ...> usage in the file (symbol: Command)
and replace the inline className value with the constant, defining that constant
above the component so future edits only change the single variable rather than
the long attribute string.
frontend/src/components/ui/alert.tsx (1)

13-15: Remove duplicate variant utilities to reduce class noise.

[&>svg]:text-current is already in the base class, and border is also already present globally. Keeping these duplicates in variant strings isn’t harmful, but removing them will make maintenance clearer.

♻️ Suggested cleanup
         destructive:
-          "bg-card text-destructive *:data-[slot=alert-description]:text-destructive/90 [&>svg]:text-current",
+          "bg-card text-destructive *:data-[slot=alert-description]:text-destructive/90",
         warning:
-          "border-warning-foreground border text-warning-foreground *:data-[slot=alert-description]:text-warning-foreground/90 [&>svg]:text-current",
+          "border-warning-foreground text-warning-foreground *:data-[slot=alert-description]:text-warning-foreground/90",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/components/ui/alert.tsx` around lines 13 - 15, Remove duplicate
utilities from the alert variants: in the Alert component's variant definitions
(the base class string and the variant map keys like "warning" and
"destructive"), eliminate redundant "[&>svg]:text-current" from the "warning"
and "destructive" variant values and remove the redundant "border" from the
"warning" variant since those are already present in the shared/base class;
update the variant strings accordingly so they only contain unique utilities.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/src/components/ui/alert-dialog.tsx`:
- Around line 131-145: The Tailwind selector in AlertDialogMedia's className is
invalid; replace the malformed `*:[svg:not([class*='size-'])]:size-8` with the
correct descendant form `[&_svg:not([class*='size-'])]:size-8` inside the
cn(...) call in the AlertDialogMedia component so child SVGs without a "size-"
class get size-8 applied.

In `@frontend/src/components/ui/field.tsx`:
- Around line 203-209: Replace the loose equality check and redundant optional
chaining for the deduped errors array: in the block that builds uniqueErrors
(the constant uniqueErrors created from new Map(...).values()), change the
conditional if (uniqueErrors?.length == 1) to use strict equality and no
optional chaining — i.e. reference uniqueErrors directly and use ===; keep
returning uniqueErrors[0]?.message (you may keep the inner optional chaining on
the message itself). This updates the check around uniqueErrors to if
(uniqueErrors.length === 1) while leaving the rest of the dedup logic unchanged.

In `@frontend/src/components/ui/popover.tsx`:
- Around line 56-63: PopoverTitle currently declares props as
React.ComponentProps<"h2"> but returns a <div>, causing a type mismatch; fix by
making the rendered element match the prop type: change the returned element to
an <h2> (keeping data-slot="popover-title", className={cn("font-medium",
className)} and {...props}) so h2-specific props are applied correctly, or
alternatively change the prop type to React.ComponentProps<"div"> if you intend
to keep a div—update the signature and ensure className and {...props} stay
intact on the chosen element.

---

Outside diff comments:
In `@frontend/package.json`:
- Around line 25-41: The package.json shows both the consolidated "radix-ui"
package and many individual "@radix-ui/*" packages; resolve this by either (A)
migrating the two remaining imports in frontend/src/components/stepper.tsx and
frontend/src/components/ui/custom/circle-progress.tsx to import from "radix-ui"
and then remove the now-unneeded individual "@radix-ui/*" entries from
package.json, or (B) revert/remove the consolidated "radix-ui" dependency and
keep the existing individual "@radix-ui/*" entries, ensuring import paths in
stepper.tsx and circle-progress.tsx match the chosen approach; update
package.json and run install to verify no unresolved imports.

---

Nitpick comments:
In `@frontend/src/components/ui/alert.tsx`:
- Around line 13-15: Remove duplicate utilities from the alert variants: in the
Alert component's variant definitions (the base class string and the variant map
keys like "warning" and "destructive"), eliminate redundant
"[&>svg]:text-current" from the "warning" and "destructive" variant values and
remove the redundant "border" from the "warning" variant since those are already
present in the shared/base class; update the variant strings accordingly so they
only contain unique utilities.

In `@frontend/src/components/ui/command.tsx`:
- Line 53: The Command component's long inline className string should be
extracted to a named constant (e.g., COMMAND_BASE_CLASSES or commandClasses) or
a cva utility to improve maintainability; locate the <Command ...> usage in the
file (symbol: Command) and replace the inline className value with the constant,
defining that constant above the component so future edits only change the
single variable rather than the long attribute string.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e80baa84-97a7-4183-8282-c11cf91ceae1

📥 Commits

Reviewing files that changed from the base of the PR and between 9719f66 and d9531ba.

⛔ Files ignored due to path filters (1)
  • frontend/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (38)
  • frontend/components.json
  • frontend/package.json
  • frontend/src/components/ui/accordion.tsx
  • frontend/src/components/ui/alert-dialog.tsx
  • frontend/src/components/ui/alert.tsx
  • frontend/src/components/ui/avatar.tsx
  • frontend/src/components/ui/badge.tsx
  • frontend/src/components/ui/badgeVariants.tsx
  • frontend/src/components/ui/breadcrumb.tsx
  • frontend/src/components/ui/button.tsx
  • frontend/src/components/ui/buttonVariants.tsx
  • frontend/src/components/ui/calendar.tsx
  • frontend/src/components/ui/card.tsx
  • frontend/src/components/ui/carousel.tsx
  • frontend/src/components/ui/checkbox.tsx
  • frontend/src/components/ui/command.tsx
  • frontend/src/components/ui/dialog.tsx
  • frontend/src/components/ui/dropdown-menu.tsx
  • frontend/src/components/ui/field.tsx
  • frontend/src/components/ui/input.tsx
  • frontend/src/components/ui/label.tsx
  • frontend/src/components/ui/navigation-menu.tsx
  • frontend/src/components/ui/pagination.tsx
  • frontend/src/components/ui/popover.tsx
  • frontend/src/components/ui/progress.tsx
  • frontend/src/components/ui/radio-group.tsx
  • frontend/src/components/ui/select.tsx
  • frontend/src/components/ui/separator.tsx
  • frontend/src/components/ui/sheet.tsx
  • frontend/src/components/ui/sidebar.tsx
  • frontend/src/components/ui/skeleton.tsx
  • frontend/src/components/ui/sonner.tsx
  • frontend/src/components/ui/switch.tsx
  • frontend/src/components/ui/table.tsx
  • frontend/src/components/ui/tabs.tsx
  • frontend/src/components/ui/textarea.tsx
  • frontend/src/components/ui/tooltip.tsx
  • frontend/src/hooks/use-mobile.ts
💤 Files with no reviewable changes (1)
  • frontend/src/hooks/use-mobile.ts

- Alby: lighter primary gradient, 1px stroke, stacked focus ring
- Alby: neutral gray palette; dark primary button label matches light
- Hub logo: default/alby light & dark fills; mono for other themes
- Figtree for alby; map tailwind font-sans via --app-font-sans
- Link buttons: data-slot/data-variant for themed primary styles

Made-with: Cursor
@stackingsaunter stackingsaunter changed the title chore(frontend): upgrade shadcn/ui components for Tailwind v4 chore(frontend): shadcn v4 upgrade, Alby theme polish, hub logo Mar 20, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/src/components/ExternalLink.tsx`:
- Around line 5-12: The Props type currently exposes all AnchorHTMLAttributes
allowing callers to override component-owned attributes and to pass anchor-only
props into the non-HTTP <span> branch; change Props to explicitly omit "target",
"rel", and "onClick" (and keep omitting "href"|"className"|"children") from the
forwarded attributes so those are owned by the component (e.g. Props =
React.PropsWithChildren<{to: string; className?: string; onClick?:
React.MouseEventHandler}>&
Omit<React.AnchorHTMLAttributes<HTMLAnchorElement>,'href'|'className'|'children'|'target'|'rel'|'onClick'>),
and when rendering the non-HTTP branch use appropriate HTMLElement attributes
(React.HTMLAttributes<HTMLSpanElement>) rather than anchor-only attributes;
ensure the component applies its internal target/rel/onClick after the spread
or, better, relies on the typing omission so callers cannot override
target/rel/onClick for ExternalLink.

In `@frontend/src/components/icons/AlbyHubLogo.tsx`:
- Around line 61-70: The logic in logoPalette incorrectly uses invert ||
isDarkMode so invert forces the dark palette regardless of current mode; update
the darkAppearance calculation in logoPalette to flip relative to the current
mode (use an XOR-style check: darkAppearance = isDarkMode !== invert or
equivalently darkAppearance = invert ? !isDarkMode : isDarkMode) so that when
invert is true the palette is the opposite of isDarkMode and you still return
"default-alby-dark" or "default-alby-light" accordingly; leave the
isDefaultOrAlbyTheme early return unchanged.

In `@frontend/src/themes/default.css`:
- Around line 8-10: The root CSS variable --app-font-sans currently falls back
to serif; update its fallback to a proper sans-serif stack so Tailwind's
font-sans (which now references --app-font-sans) doesn't revert to serif when
"Inter var" is unavailable—replace the trailing serif fallback with a sensible
sans stack (e.g. system UI and common sans fonts) in the --app-font-sans
declaration.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4ca21131-240d-42f2-b870-b00baa974ffa

📥 Commits

Reviewing files that changed from the base of the PR and between d9531ba and ddb214b.

⛔ Files ignored due to path filters (1)
  • frontend/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (10)
  • frontend/package.json
  • frontend/src/components/AppSidebar.tsx
  • frontend/src/components/ExternalLink.tsx
  • frontend/src/components/icons/AlbyHubLogo.tsx
  • frontend/src/components/layouts/TwoColumnFullScreenLayout.tsx
  • frontend/src/components/ui/custom/external-link-button.tsx
  • frontend/src/components/ui/custom/link-button.tsx
  • frontend/src/themes/alby.css
  • frontend/src/themes/default.css
  • frontend/src/themes/index.css
✅ Files skipped from review due to trivial changes (1)
  • frontend/src/themes/index.css
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/package.json

Select Xcode.app for CGO on macOS so Wails WebKit compiles.

Skip Apple signing/notarize/DMG when the PR head is a fork (no secrets);
upload an unsigned zip for Wails fork PRs.

Set fail-fast false on the HTTP build matrix so Linux jobs finish if macOS
fails.

Made-with: Cursor
For workflow_call runs, require inputs.build-release for macOS signing.
Fork PR logic unchanged.

Made-with: Cursor
Revert workflow_call gating and fork unsigned logic.
Remove Xcode selection step added for CGO builds.

Made-with: Cursor
@reneaaron reneaaron marked this pull request as draft March 24, 2026 14:48
@reneaaron
Copy link
Contributor

Please don't mix 10 different things into one PR, this is impossible to review. (especially any updates to libraries should be separate from any features)

@reneaaron reneaaron marked this pull request as ready for review March 25, 2026 20:08
Copy link
Contributor

@reneaaron reneaaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took a close look and fixed / improved a lot of things that were just very hard to maintain or not consistent overall.

Overall some nice changes, the Alby theme with the updated fonts / buttons for sure looks nice! 💯

tACK

"prepare": "cd .. && husky frontend/.husky"
},
"dependencies": {
"@fontsource-variable/figtree": "^5.2.10",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that's cool :) Never heard about that before...

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/src/themes/alby.css`:
- Around line 92-123: The three complex selectors using chained :not() (for
example :is(button, a,
span)[data-slot="button"][data-variant="default"]:not(:disabled):not([aria-disabled="true"])
and their :hover and :focus-visible variants) violate the stylelint
"selector-not-notation: complex" rule; replace chained :not() calls with a
single :not() that contains a comma-separated list like :not(:disabled,
[aria-disabled="true"]) in each selector (update the base selector, its :hover
and its :focus-visible variants accordingly) so each selector uses the complex
:not(:disabled, [aria-disabled="true"]) form.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4f70fc1f-364a-4ac3-afa0-e84cc8e31609

📥 Commits

Reviewing files that changed from the base of the PR and between ddb214b and f3e21fd.

⛔ Files ignored due to path filters (3)
  • frontend/public/fonts/Inter-italic.var.woff2 is excluded by !**/*.woff2
  • frontend/public/fonts/Inter-roman.var.woff2 is excluded by !**/*.woff2
  • frontend/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (10)
  • frontend/package.json
  • frontend/src/components/AppSidebar.tsx
  • frontend/src/components/icons/AlbyHubLogo.tsx
  • frontend/src/components/layouts/TwoColumnFullScreenLayout.tsx
  • frontend/src/components/ui/sonner.tsx
  • frontend/src/fonts.css
  • frontend/src/main.tsx
  • frontend/src/themes/alby.css
  • frontend/src/themes/default.css
  • frontend/src/themes/index.css
💤 Files with no reviewable changes (2)
  • frontend/src/main.tsx
  • frontend/src/fonts.css
✅ Files skipped from review due to trivial changes (1)
  • frontend/src/components/AppSidebar.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
  • frontend/src/themes/index.css
  • frontend/src/components/ui/sonner.tsx
  • frontend/package.json

Comment on lines +92 to +123
.theme-alby :is(button, a, span)[data-slot="button"][data-variant="default"]:not(:disabled):not(
[aria-disabled="true"]
) {
background:
linear-gradient(
180deg,
rgba(255, 255, 255, 0.58) 0%,
rgba(255, 236, 175, 0.22) 100%
),
#fff0b8;
box-shadow: 0 0 0 1px #ffdf6f;
}

--destructive: hsl(0 84% 60%);
.theme-alby
:is(button, a, span)[data-slot="button"][data-variant="default"]:hover:not(:disabled):not(
[aria-disabled="true"]
) {
background:
linear-gradient(
180deg,
rgba(255, 255, 255, 0.45) 0%,
rgba(255, 224, 140, 0.28) 100%
),
#ffe9a3;
box-shadow: 0 0 0 1px #ffdf6f;
}

--border: hsl(0 0% 15%);
--input: hsl(0 0% 15%);
--ring: hsl(47 100% 40%);
/* Keep keyboard focus ring (Button uses ring via box-shadow); stack with brand stroke. */
.theme-alby
:is(button, a, span)[data-slot="button"][data-variant="default"]:focus-visible:not(:disabled):not(
[aria-disabled="true"]
) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and examine the relevant lines
fd "alby.css" --type f

Repository: getAlby/hub

Length of output: 83


🌐 Web query:

Stylelint selector-not-notation rule CSS :not() pseudo-class behavior

💡 Result:

The Stylelint rule "selector-not-notation" controls the notation used for the CSS :not pseudo-class selectors, enforcing either "simple" or "complex" notation based on Selectors Level 3 vs. Level 4 specifications. "simple" (Selectors Level 3, backwards compatible): Allows only a single simple selector inside :not, e.g. :not(a), :not(.foo). Disallows complex selectors like :not(a.foo) or lists like :not(a, div). Multiple conditions must use chained :nots, e.g. :not(a):not(div), which affects specificity (e.g. 0,2,1 vs. 0,1,1 for complex). "complex" (Selectors Level 4, modern): Allows complex selectors like :not(a.foo) and selector lists like :not(a, div), :not(a).foo:not(:empty). The rule is autofixable for most cases, converting between notations (with caveats for specificity changes). Configuration examples: { "selector-not-notation": "simple" } { "selector-not-notation": "complex" }

Citations:


🏁 Script executed:

# Examine lines 92-123 of alby.css
sed -n '92,123p' frontend/src/themes/alby.css

Repository: getAlby/hub

Length of output: 975


🏁 Script executed:

# Find stylelint configuration files
fd "stylelint" -type f
fd ".stylelintrc" -type f
find . -maxdepth 2 -name "*stylelint*" -type f

Repository: getAlby/hub

Length of output: 433


🏁 Script executed:

# Read the stylelint configuration
cat ./.stylelintrc.json

Repository: getAlby/hub

Length of output: 1061


🌐 Web query:

stylelint-config-standard-scss selector-not-notation default value

💡 Result:

The default value for the "selector-not-notation" rule in stylelint-config-standard-scss is "complex".

Citations:


Use the complex :not() form so Stylelint passes.

These three selectors violate the selector-not-notation: complex rule (the default in stylelint-config-standard-scss). The complex notation requires combining multiple selectors in a single :not() with a comma-separated list, not chained :not() pseudo-classes.

💡 Suggested selector rewrite
-  .theme-alby :is(button, a, span)[data-slot="button"][data-variant="default"]:not(:disabled):not(
-      [aria-disabled="true"]
-    ) {
+  .theme-alby :is(button, a, span)[data-slot="button"][data-variant="default"]:not(
+      :disabled,
+      [aria-disabled="true"]
+    ) {
@@
-    :is(button, a, span)[data-slot="button"][data-variant="default"]:hover:not(:disabled):not(
-      [aria-disabled="true"]
-    ) {
+    :is(button, a, span)[data-slot="button"][data-variant="default"]:hover:not(
+      :disabled,
+      [aria-disabled="true"]
+    ) {
@@
-    :is(button, a, span)[data-slot="button"][data-variant="default"]:focus-visible:not(:disabled):not(
-      [aria-disabled="true"]
-    ) {
+    :is(button, a, span)[data-slot="button"][data-variant="default"]:focus-visible:not(
+      :disabled,
+      [aria-disabled="true"]
+    ) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.theme-alby :is(button, a, span)[data-slot="button"][data-variant="default"]:not(:disabled):not(
[aria-disabled="true"]
) {
background:
linear-gradient(
180deg,
rgba(255, 255, 255, 0.58) 0%,
rgba(255, 236, 175, 0.22) 100%
),
#fff0b8;
box-shadow: 0 0 0 1px #ffdf6f;
}
--destructive: hsl(0 84% 60%);
.theme-alby
:is(button, a, span)[data-slot="button"][data-variant="default"]:hover:not(:disabled):not(
[aria-disabled="true"]
) {
background:
linear-gradient(
180deg,
rgba(255, 255, 255, 0.45) 0%,
rgba(255, 224, 140, 0.28) 100%
),
#ffe9a3;
box-shadow: 0 0 0 1px #ffdf6f;
}
--border: hsl(0 0% 15%);
--input: hsl(0 0% 15%);
--ring: hsl(47 100% 40%);
/* Keep keyboard focus ring (Button uses ring via box-shadow); stack with brand stroke. */
.theme-alby
:is(button, a, span)[data-slot="button"][data-variant="default"]:focus-visible:not(:disabled):not(
[aria-disabled="true"]
) {
.theme-alby :is(button, a, span)[data-slot="button"][data-variant="default"]:not(
:disabled,
[aria-disabled="true"]
) {
background:
linear-gradient(
180deg,
rgba(255, 255, 255, 0.58) 0%,
rgba(255, 236, 175, 0.22) 100%
),
`#fff0b8`;
box-shadow: 0 0 0 1px `#ffdf6f`;
}
.theme-alby
:is(button, a, span)[data-slot="button"][data-variant="default"]:hover:not(
:disabled,
[aria-disabled="true"]
) {
background:
linear-gradient(
180deg,
rgba(255, 255, 255, 0.45) 0%,
rgba(255, 224, 140, 0.28) 100%
),
`#ffe9a3`;
box-shadow: 0 0 0 1px `#ffdf6f`;
}
/* Keep keyboard focus ring (Button uses ring via box-shadow); stack with brand stroke. */
.theme-alby
:is(button, a, span)[data-slot="button"][data-variant="default"]:focus-visible:not(
:disabled,
[aria-disabled="true"]
) {
🧰 Tools
🪛 Stylelint (17.4.0)

[error] 92-94: Expected complex :not() pseudo-class notation (selector-not-notation)

(selector-not-notation)


[error] 106-108: Expected complex :not() pseudo-class notation (selector-not-notation)

(selector-not-notation)


[error] 121-123: Expected complex :not() pseudo-class notation (selector-not-notation)

(selector-not-notation)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/themes/alby.css` around lines 92 - 123, The three complex
selectors using chained :not() (for example :is(button, a,
span)[data-slot="button"][data-variant="default"]:not(:disabled):not([aria-disabled="true"])
and their :hover and :focus-visible variants) violate the stylelint
"selector-not-notation: complex" rule; replace chained :not() calls with a
single :not() that contains a comma-separated list like :not(:disabled,
[aria-disabled="true"]) in each selector (update the base selector, its :hover
and its :focus-visible variants accordingly) so each selector uses the complex
:not(:disabled, [aria-disabled="true"]) form.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
frontend/src/components/stepper.tsx (1)

235-237: Widen the Panel ref node type for polymorphic asChild rendering.

The Panel component supports the asChild prop, which allows rendering via Slot.Root (a polymorphic component). When asChild=true, non-div elements can be rendered, making HTMLDivElement | null overly specific. Widen both the ref callback and scrollIntoStepperPanel to accept HTMLElement | null for type accuracy and forward-compatibility, since scrollIntoView() works on any element type.

Proposed change
-            ref={(node: HTMLDivElement | null) =>
+            ref={(node: HTMLElement | null) =>
               scrollIntoStepperPanel(node, tracking)
             }
-function scrollIntoStepperPanel(
-  node: HTMLDivElement | null,
+function scrollIntoStepperPanel(
+  node: HTMLElement | null,
   tracking?: boolean
 ) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/components/stepper.tsx` around lines 235 - 237, The ref callback
currently types the node as HTMLDivElement | null which is too specific for
polymorphic asChild rendering; update the ref signature and the
scrollIntoStepperPanel function to accept HTMLElement | null instead of
HTMLDivElement | null so non-div elements (e.g., Slot.Root) are supported and
scrollIntoView can be called; specifically change the ref={(node: HTMLDivElement
| null) => scrollIntoStepperPanel(node, tracking)} typing and the
scrollIntoStepperPanel parameter type to HTMLElement | null (preserving null
handling).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@frontend/src/components/stepper.tsx`:
- Around line 235-237: The ref callback currently types the node as
HTMLDivElement | null which is too specific for polymorphic asChild rendering;
update the ref signature and the scrollIntoStepperPanel function to accept
HTMLElement | null instead of HTMLDivElement | null so non-div elements (e.g.,
Slot.Root) are supported and scrollIntoView can be called; specifically change
the ref={(node: HTMLDivElement | null) => scrollIntoStepperPanel(node,
tracking)} typing and the scrollIntoStepperPanel parameter type to HTMLElement |
null (preserving null handling).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8f5a2f47-cf90-4d10-b25b-eb7745bed61b

📥 Commits

Reviewing files that changed from the base of the PR and between f3e21fd and a0d7c49.

⛔ Files ignored due to path filters (1)
  • frontend/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (3)
  • frontend/package.json
  • frontend/src/components/stepper.tsx
  • frontend/src/components/ui/custom/circle-progress.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/package.json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants